Refactor pattern shaders to use OpenMPT numeric note encoding#112
Merged
Refactor pattern shaders to use OpenMPT numeric note encoding#112
Conversation
…te-on/volume indicators
Audit findings and fixes:
1. v0.42.wgsl — complete rewrite (was full-screen quad, showed zero pattern data):
- Convert to per-cell instancing (circular ring layout, 64-step arcs)
- instanceIndex >= totalCells renders ring-grid background pass
- instanceIndex < totalCells renders per-cell quads with note data
- Add cells[] / channels[] / rowFlags bindings
- Blue micro-LED (top-left): note-on indicator (lights when playhead hits)
- Amber micro-LED (top-right): volume command indicator (volCmd > 0)
- Green micro-LED (bottom): effect command indicator (effCmd > 0)
- Red/orange tint for note-off (121) and note-cut (122/123)
2. v0.45.wgsl — fix critical note detection bug:
- REMOVE ASCII range check (noteChar >= 65 && <= 122) which missed notes 1-64
(roughly C-0 through E-4, including middle-C / C-4 = note 60)
- FIX pitchClassFromPacked: replace ASCII switch table with correct numeric
formula: (note - 1) % 12 (OpenMPT internal: note 1 = C-0, note 13 = C-1…)
- Add note-off (121) and note-cut/fade (122/123) visual distinction
- Add same three micro-LED indicators as v0.42
3. v0.38.wgsl — targeted fixes:
- Fix hasNote: (note > 0u) → (note > 0u && note <= 120u) to exclude 121-123
- Add isNoteOff/isNoteCut rendering in main LED (dim red / orange)
- Change top LED from cyan to amber to consistently indicate volume/expression
4. PatternDisplay.tsx:
- Add v0.42 to isHighPrec list (was using old standard packing, missing volCmd)
- Add v0.42 to isCircularLayoutShader (affects bezel uniform scaling)
Format compliance note: libopenmpt returns OpenMPT internal note numbers (0-123)
directly. Shaders must use numeric comparisons, not ASCII character ranges.
https://claude.ai/code/session_01K5XYz1JJ1ppmmfcCGPzz9w
Critical fixes from shader visual audit:
1. v0.45.wgsl — Remove early discard clipping 30% of cells:
- Line 221-223: Removed `if (!isCap) { discard; }` that clipped all cells outside frosted cap bounds
- Replace with alpha masking via capEdge (smoothstep on SDF)
- Allows all pattern cells to render with proper falloff at edges
2. v0.47.wgsl — Fix instrument byte unpacking:
- Line 357: Changed `inst = in.packedA & 255u` → `(in.packedA >> 16) & 255u`
- Was reading lowest byte (value field) instead of bits 16-23
- Also fixed note check: ASCII A-G (65-71) → numeric 1-120 range
- Now displays all notes correctly and brightness calculation works
3. v0.46.wgsl & v0.49.wgsl — Fix derivative undefined behavior:
- Moved early `if (in.position.y > canvasH * 0.88) { discard; }` to AFTER playheadActivation computation
- Early discard before fwidth() was called created non-uniform control flow
- Could cause undefined behavior in derivative calculations
- Now safe: discard only after all derivatives consumed
4. v0.45.wgsl — Verified playheadActivation scope:
- Computed at line 240, used safely throughout at 255-309 ✓
Impact: v0.45/46/47/49 now render complete pattern data without clipping or missing notes.
https://claude.ai/code/session_01K5XYz1JJ1ppmmfcCGPzz9w
Prevent undefined buffer access in v0.47/48/49/50:
All circular layout shaders that access channels[in.channel] now include bounds checks
before array access. This prevents GPU crashes or silent data corruption when:
- padTopChannel is enabled (shifts music channels 1-32, adds dummy channel 0)
- numChannels array length doesn't match expected size
Pattern: Changed `let ch = channels[in.channel];` to:
```
var ch = ChannelState(0.0, 0.0, 0.0, 0u, 1000.0, 0u, 0.0, 0u);
if (in.channel < arrayLength(&channels)) {
ch = channels[in.channel];
}
```
v0.45 and v0.39 already had correct bounds checks.
v0.46 is overlay-only and doesn't access channels[].
This fixes the CRITICAL undefined behavior identified in shader audit.
https://claude.ai/code/session_01K5XYz1JJ1ppmmfcCGPzz9w
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unified pattern shader implementations to use OpenMPT's numeric note encoding (1–120 for notes, 121 for OFF, 122–123 for CUT/FADE) instead of ASCII character parsing. This simplifies note detection logic and improves consistency across shader versions.
Key Changes
patternv0.42.wgsl (Major Rewrite)
patternv0.45.wgsl
(note - 1) % 12patternv0.38.wgsl
(note > 0u)to(note > 0u) && (note <= 120u)to properly handle OpenMPT encodingPatternDisplay.tsx
Implementation Details
(note - 1) % 12yields 0–11 for C through Bhttps://claude.ai/code/session_01K5XYz1JJ1ppmmfcCGPzz9w